Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added dynamic throttling of events to live/dynamic widgets #596

Merged
merged 6 commits into from Apr 11, 2016

Conversation

philippjfr
Copy link
Member

This PR adds dynamic throttling of events to the widgets. The first time a widget dynamically requests a frame from the Python kernel it will instantiate this.time, which will then be used to calculate the time taken to generate that frame. Each subsequent time a new frame is requested the dynamic_update method will first check whether sufficient time has passed since the last frame was requested, if not it will queue the event. Whenever a callback completes it will execute the last queued event if there is any. This approach ensures that events generated faster than they can be processed are thrown away, while still ensuring that the last event that the user generated is processed, which means that the slider and the plot stay in sync.

Here's what I used for testing:

def a(i):
    time.sleep(1)
    return hv.Image(np.random.rand(10,10))

hv.DynamicMap(a, kdims=[hv.Dimension('x', range=(0, 1.))])

This also let me measure the speed of plotting of updates for the first time. For this example, if you disable the sleep, I'm getting ~30 ms per frame in matplotlib and ~35ms per frame using bokeh.

@philippjfr philippjfr changed the title Addded dynamic throttling of events to live/dynamic widgets Added dynamic throttling of events to live/dynamic widgets Apr 8, 2016
@philippjfr
Copy link
Member Author

Doesn't yet work very well for the scrubber widget. Will have to think about how to deal with that.

@philippjfr
Copy link
Member Author

Okay, I've now added handling for the scrubber widget as well and made some fixes to the queuing. The scrubber widget now simply waits until a frame is ready to request a new one on play.

@philippjfr
Copy link
Member Author

This is ready to review now, it would be best if whoever reviews it also tries it out. I've tested all the permutations of live, dynamic and embedded widget with matplotlib and bokeh but since there are quite a few changes I'd feel more comfortable if someone independently confirmed things are still working as expected.

@jlstevens
Copy link
Contributor

I'm happy to review it but in order to spend some testing it and making sure it is ok, it probably won't be merged before Monday.

@jlstevens
Copy link
Contributor

As promised, I'm now testing this PR. It is certainly a huge improvement to responsiveness!

There is one issue I've noticed though (tested on JupyterHub). Trying the DynamicMap tutorial, I am using huge arrays to slow things down:

x,y = np.mgrid[-1000:1001, -1000:1001] * 0.1

def sine_array(phase, freq):
    return np.sin(phase + (freq*x**2+freq*y**2))

def sine_image(phase, freq):
    return hv.Image(np.sin(phase + (freq*x**2+freq*y**2)))

dmap = hv.DynamicMap(sine_image, kdims=[hv.Dimension('phase',range=(0, np.pi)),
                                 hv.Dimension('frequency', range=(0.01,np.pi))])
dmap

I notice the throttling but I notice I can drag the slider quickly to the end (to np.pi) and the final image doesn't show that - it shows the image at some intermediate point. In other words, I see this:

screen shot 2016-04-11 at 3 46 25 pm

When I think it ought to be something more like this:

screen shot 2016-04-11 at 3 47 24 pm

@philippjfr
Copy link
Member Author

I notice the throttling but I notice I can drag the slider quickly to the end (to np.pi) and the final image doesn't show that - it shows the image at some intermediate point. In other words, I see this:

Seems to be a completely unrelated issue with the Dimension ranges, somewhere I assume is checking whether the value is out of bounds with val < dim.range[1] which fails in this case. So whenever you hit the upper bound an exception is raised which clears the event queue. I think you'll see exactly the same behavior without this PR, so I still think this can be merged asap.

@jlstevens
Copy link
Contributor

Well at least we know what is going wrong in this case!

Given my tests worked well (except for the example above) and everything is certainly more responsive without all the events queuing up, I will merge this PR now.

@jlstevens jlstevens merged commit e45cbaa into master Apr 11, 2016
@philippjfr philippjfr added this to the v1.5.0 milestone Apr 20, 2016
@philippjfr philippjfr deleted the dynamic_throttle branch April 20, 2016 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants